[2/n] handle unparseable local files (e.g., conflict markers) #39
+2,137
−150
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.6-beta.1
There was a problem hiding this comment.
Pull request overview
This PR enhances the Dropshot API manager to gracefully handle unparseable local OpenAPI files, such as those containing merge conflict markers from Git or Jujutsu. The tool now detects corrupted files, tracks them as problems, and provides automatic regeneration from blessed content.
Key changes:
- Introduces tracking for unparseable files through a new
LocalApiSpecFile::Unparseablevariant andUnparseableFilestruct - Adds new problem types (
UnparseableLocalFile,BlessedVersionCorruptedLocal) and fix types (RegenerateFromBlessed,DeleteUnparseableFile) to handle corrupted files - Implements MERGE_HEAD detection in git merge base calculation to ensure blessed documents from incoming branches are visible during merge conflicts
- Adds comprehensive integration tests covering merge and rebase scenarios for both git and jj, including conflict resolution workflows
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/integration-tests/tests/integration/git_ref.rs | Adds comprehensive integration tests for merge/rebase scenarios with git and jj, testing conflict resolution when git refs are involved |
| crates/integration-tests/src/lib.rs | Exports new test result types and helper functions for merge/rebase testing |
| crates/integration-tests/src/fixtures.rs | Adds new API fixtures (v1_v2_v4, v3_alternate, v1_v2_v3_v4alt) for testing merge conflict scenarios |
| crates/integration-tests/src/environment.rs | Adds merge/rebase result enums and helper methods for git and jj operations, including conflict detection |
| crates/dropshot-api-manager/src/spec_files_local.rs | Converts LocalApiSpecFile to an enum with Valid and Unparseable variants to track corrupted files |
| crates/dropshot-api-manager/src/spec_files_generic.rs | Adds UnparseableFile struct, SpecFileInfo trait, and updates ApiLoad trait to support unparseable files |
| crates/dropshot-api-manager/src/spec_files_generated.rs | Implements new ApiLoad trait methods for generated files (unparseable files not allowed) |
| crates/dropshot-api-manager/src/spec_files_blessed.rs | Implements new ApiLoad trait methods for blessed files (unparseable files not allowed) |
| crates/dropshot-api-manager/src/resolved.rs | Adds logic to detect, track, and regenerate corrupted local files; includes new Problem and Fix variants |
| crates/dropshot-api-manager/src/git.rs | Updates git_merge_base_head to use MERGE_HEAD during active merges for correct blessed document resolution |
| crates/dropshot-api-manager/src/cmd/debug.rs | Updates debug output to handle unparseable files gracefully |
| .github/workflows/ci.yml | Adds jj-cli installation for running jj integration tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Created using spr 1.3.6-beta.1
Collaborator
Author
Manual test notesWithout git ref storage enabledBoth branches add v3 but with different implementations.
Setup# Create isolated test environment.
rm -rf /tmp/merge-test && mkdir /tmp/merge-test
cp -r /home/rain/dev/oxide/dropshot-api-manager /tmp/merge-test/workspace
cd /tmp/merge-test/workspace
rm -rf .git .jj target
git init && git add -A && git commit -m "initial"
# Create baseline (v1, v2 only)—edit lib.rs to remove v3, revert ThingV2 to u32.
cargo example-openapi generate --blessed-from-dir e2e-example/documents
git add -A && git commit -m "baseline: v1, v2 only"
# Create branch_a: v3 with Number wrapper.
git checkout -b branch_a
# Edit lib.rs: add v3, change ThingV2 to use Number wrapper.
cargo example-openapi generate --blessed-from-git main
git add -A && git commit -m "add v3 (implementation A: Number wrapper)"
# Produces: versioned-3.0.0-b48dc9.json
# Create branch_b: v3 with /status endpoint.
git checkout main && git checkout -b branch_b
# Edit lib.rs: add v3, add get_status endpoint and StatusResponse.
cargo example-openapi generate --blessed-from-git main
git add -A && git commit -m "add v3 (implementation B: /status endpoint)"
# Produces: versioned-3.0.0-e47ffc.json (different hash)Git merge testgit merge branch_a
# Conflicts: lib.rs and versioned-latest.json.
# Both versioned-3.0.0-b48dc9.json and versioned-3.0.0-e47ffc.json exist.
# Manually resolve lib.rs: merge both implementations (A's v3 stays, B's becomes v4).
cargo example-openapi check --blessed-from-git mainCheck output before generate: cargo example-openapi generate --blessed-from-git mainGenerate output: cargo example-openapi check --blessed-from-git mainCheck output after generate: git add -A && git commit -m "merge: A's v3 + B's v3 becomes v4"jj merge test# Reset and init jj.
git checkout main && git branch -D branch_a branch_b
git reset --hard <baseline-commit>
jj git init --colocate
# Create branch_a with v3 (implementation A).
# Edit lib.rs: add v3 with Number wrapper.
cargo example-openapi generate --blessed-from-git main
jj commit -m "add v3 (implementation A)" && jj bookmark create branch_a -r @-
# Create branch_b with v3 (implementation B).
jj new main -m "add v3 (implementation B)"
# Edit lib.rs: add v3 with /status endpoint.
cargo example-openapi generate --blessed-from-git main
jj bookmark create branch_b
# Merge.
jj new branch_a branch_b -m "merge"
# Reports conflict on lib.rs and symlink.cat e2e-example/documents/versioned/versioned-latest.json
# jj converted symlink to regular file with conflict markers.# Manually resolve lib.rs.
cargo example-openapi check --blessed-from-git mainCheck output before generate (jj): cargo example-openapi generate --blessed-from-git mainGenerate output (jj): cargo example-openapi check --blessed-from-git main
# Success: 5 documents checked, 6 fresh, 0 stale, 0 failed, 0 other problems.
jj status # No conflict marker.With git ref storage enabledSetup# Create isolated test environment.
rm -rf /tmp/merge-test && mkdir /tmp/merge-test
cp -r /home/rain/dev/oxide/dropshot-api-manager /tmp/merge-test/workspace
cd /tmp/merge-test/workspace
rm -rf .git .jj target
git init && git add -A && git commit -m "initial"
# Enable git ref storage in e2e-example/bin/src/main.rs.
# Uncomment .with_git_ref_storage().
# Create baseline with v1, v2, v3.
cargo example-openapi generate --blessed-from-dir e2e-example/documents
git add -A && git commit -m "enable git ref storage; baseline v1, v2, v3"
# Add v4 to main (implementation A: /status endpoint).
# Edit lib.rs: add v4 with StatusResponse and /status endpoint.
cargo example-openapi generate --blessed-from-git main
git add -A && git commit -m "add v4 (implementation A: /status endpoint)"
# v1, v2, v3 converted to .gitref files; v4 is JSON.
# Create branch_b from baseline (before v4), add different v4.
git checkout -b branch_b <baseline-commit>
# Edit lib.rs: add v4 with Metadata and /metadata endpoint.
cargo example-openapi generate --blessed-from-git <baseline-commit>
git add -A && git commit -m "add v4 (implementation B: /metadata endpoint)"
# v1, v2, v3 converted to .gitref files; v4 is JSON (different hash).Git merge testgit merge main
# Reports:
# conflict (content): e2e-example/apis/src/lib.rs
# conflict (rename/rename): versioned-3.0.0-b48dc9.json renamed to
# versioned-4.0.0-f05d45.json in HEAD and to versioned-4.0.0-dc7291.json in main
# conflict (content): versioned-latest.jsonGit detects a rename/rename conflict because both branches renamed # After manually resolving lib.rs: A's v4 stays as v4, B's becomes v5.
cargo example-openapi check --blessed-from-git mainCheck output before generate: cargo example-openapi generate --blessed-from-git mainGenerate output: cargo example-openapi check --blessed-from-git main
# Success: 6 documents checked, 7 fresh, 0 stale, 0 failed, 0 other problems.
git add -A && git commit -m "merge: main's v4 + branch_b's v4 becomes v5"jj merge test# Reset and init jj.
git reset --hard <baseline-commit>
jj git init --colocate
# Add v4 on main (implementation A).
# Edit lib.rs: add v4 with /status endpoint.
cargo example-openapi generate --blessed-from-git main
jj commit -m "add v4 (implementation A)" && jj bookmark set main -r @-
# Create branch_b from baseline with v4 (implementation B).
jj new <baseline> -m "add v4 (implementation B)"
# Edit lib.rs: add v4 with /metadata endpoint.
cargo example-openapi generate --blessed-from-git <baseline>
jj bookmark create branch_b
# Merge.
jj new main branch_b -m "merge"
# Reports:
# e2e-example/apis/src/lib.rs 2-sided conflict
# versioned-latest.json 2-sided conflict including a symlinkjj does not report rename/rename conflicts, since it currently lacks merge-time cat e2e-example/documents/versioned/versioned-latest.json
# jj converted symlink to regular file with conflict markers.# Manually resolve lib.rs: A's v4 stays as v4, B's becomes v5.
cargo example-openapi check --blessed-from-git mainCheck output before generate: cargo example-openapi generate --blessed-from-git mainGenerate output: cargo example-openapi check --blessed-from-git main
# Success: 6 documents checked, 7 fresh, 0 stale, 0 failed, 0 other problems.
jj status # No conflict marker. |
Created using spr 1.3.6-beta.1
sunshowers
changed the title
Created using spr 1.3.6-beta.1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, when a local OpenAPI document could be parsed—for example, because it contained conflict markers—the Dropshot API manager failed with a "couldn't be parsed" error. With this commit, the manager now gracefully handles the situation. This becomes particularly relevant after #38, after which conflicting versions are likely to be marked as rename/rename conflicts in Git.
This commit introduces unparseable file tracking, along with these problem and fix types:
Problem::UnparseableLocalFilefor standalone unparseable files that need to be deleted.Problem::BlessedVersionCorruptedLocalfor unparseable files that match a blessed version's hash and can be regenerated.Fix::RegenerateFromBlessedregenerates a corrupted file from blessed content, optionally as a git ref.Fix::DeleteUnparseableFileremoves orphaned unparseable files.This commit is careful to only report unparseable files as problems if no other fix will overwrite them. (In those cases, regeneration will naturally clean up the corrupted file.)
For unparseable symlinks, we just treat them as missing.
Merge state awareness. During a git merge conflict, we now use MERGE_HEAD instead of HEAD when computing merge bases. This ensures blessed documents from the incoming branch are visible, which means that users won't have to do another generate run after finishing up resolution of merge conflicts.
Note that jj does not write out
MERGE_HEAD, but that is okay because conflicts are first class in jj (in other words, it doesn't have modal/intermediate states like git). This means that the actual merge base of HEAD and main will be accurate.This commit includes integration tests for:
generateresolves.Also add the same set of tests for jj. Note that jj (currently) lacks rename detection during merges/rebases, and also that it turns conflicted symlinks into regular files. We test for both these conditions.